Skip to content

Conversation

@7zrv
Copy link
Collaborator

@7zrv 7zrv commented Nov 29, 2024

📌 과제 설명

관심기관의 등록과 취소 기능 구현입니다.

👩‍💻 요구 사항과 구현 내용

  • InterestCenter 엔티티에 누락된 기관, 봉사자 id 필드 추가
  • 관심 기관 등록 유스케이스 추가및 서비스 레이어 구현
  • 등록, 검증을 위한 영속성 레이어 구현
  • 중복 예외 클래스 추가
  • 예외 메세지 추가
  • 관심 기관 취소 유스케이스 추가및 서비스 레이어 구현
  • 관심기관 취소, 검증을 위한 영속성 레이어 구현
  • 관심 기관 등록과 취소 엔드포인트 컨트롤러 구현
  • ApiResponse 공통 응답 객체 메서드 추가(논의 예정)
  • 테스트 코드 작성및 검증완료

✅ PR 포인트 & 궁금한 점

queryDSL 사용이 아직 미숙해서 해당 레포지토리 부분 잘 봐주시면 감사하겠습니다!

7zrv added 3 commits November 29, 2024 18:20
- InterestCenter 엔티티에 누락된 기관, 봉사자 id 필드 추가
- 관심 기관 등록 유스케이스 추가및 서비스 레이어 구현
- 등록, 검증을 위한 영속성 레이어 구현
- 중복 예외 클래스 추가
- 예외 메세지 추가
- 테스트 코드 작성및 검증 완
- 관심 기관 취소 유스케이스 추가및 서비스 레이어 구현
- 관심기관 취소, 검증을 위한 영속성 레이어 구현
- 예외 메세지 추가
- 테스트 코드 작성및 검증 완료
- 관심 기관 등록과 취소 엔드포인트 컨트롤러 구현
- ApiResponse 공통 응답 객체 메서드 추가(논의 예정)
- 테스트 코드 작성및 검증완료
- 관심기관 등록 요청 Dto에 유효성 검증 어노테이션 추가
- 활성화를 위해 컨트롤러에 @Valid 어노테이션 추가
@m-a-king
Copy link
Collaborator

서진 님,

로 표기해주세요 ㅎㅎ

- 기존 와일드카드를 사용하던 ApiResponse의 ok 메서드를 String 타입으로 리팩토링
- 변경에 따른 컨트롤러 return문 수정
Copy link
Collaborator

@m-a-king m-a-king left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다!

Comment on lines 22 to 23
public static ApiResponse<String> okWithoutData(String message) {
return new ApiResponse<>(200, message, "");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 이거 만들려고 했는데 ㅎㅎ. 감사히 사용하겠습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

최신 커밋에서는 이름이 다시 ok로 변경되었습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

더 좋습니다.

@@ -0,0 +1,8 @@
package com.somemore.global.exception;

public class DuplicateException extends RuntimeException{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BadReq로 모든 것을 처리하는 게 아니였나요...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중복 검사가 계속 생길거 같은데 전부 BadReq로 하면 예외가 명확하지 못한 느낌이 들어서 추가해주었습니다
전부 BadReq로 처리하기로 한 거 였나요? BadReq를 포괄적으로 사용할 수 있다 라는 느낌으로 이해하고 있었어요

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BadReq에서 구분된다면, Exception 메시지들도 따로 관리하는 것이 명확하다고 생각했고 그 반대도 같은 의미로 받아들이고 있었습니다. 저는 오히려 서진님의 구분된 Exception이 더 명확하다고 생각합니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception 메시지들도 따로 관리하는 것이 혹시 도메인 별로 메시지들을 관리하는게 맞나요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Exception이 BadReq 뿐만 아니라 여러 개로 나뉨
  2. Exception 별로 예외 메시지 클래스가 생성 -> 서로 다른 ENUM 예외 메시지 클래스
  3. ENUM 클래스 -> 도메인 별로 메시지를 관리할 수 있도록 템플릿처럼 관리 (%s ~~~~~ 입니다, 도메인)
  4. %s에 들어갈 도메인 ENUM 클래스...
  5. 3에서 만든 클래스 -> (%s%s, 유저, 닉네임), (%s%s, 기관, 전화번호?)

이거 같은데 우선 진행해도 무방하다고 생각합니다.


@RequiredArgsConstructor
@RestController
@RequestMapping("/api/interest-center")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 나중에 s를 뒤에 붙이실 때 어떻게 하실건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생각하지 못했습니다 차라리 컨트롤러에 적어주는게 나을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

범수님은 컨트롤러 엔드포인트 하나하나마다 추가해주셨습니다. 개발 시작 전에도 범수 님이 이 문제에 대해서 짚어주셔서 인지하고 있었습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀해주신 방법이 더 깔끔한 것 같아요 저도 엔드포인트마다 추가 해주는걸로 수정하겠습니다

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨트롤러 테스트는 진짜 어지럽네요...


InterestCenter savedInterestCenter = interestCenterRepository.findById(responseDto.id())
.orElseThrow(() -> new IllegalStateException("등록된 관심 기관이 없습니다."));
assertEquals(savedInterestCenter.getId(), responseDto.id());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertJ로 풀어내지 않으신 이유가 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

체이닝이 없어서 무의식적으로 assertJ 없이 했던것 같습니다

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 ㅎㅎ 다들 J 쓰시길래 저도 억지로 하나하나 리팩토링했어서 여쭤봐씁니다.

Copy link
Collaborator

@ayoung-dev ayoung-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드가 아주 깔끔한 거 같습니다!
고생하셨어요오

Copy link
Collaborator

@leebs0521 leebs0521 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다.

UUID volunteerId,

@Schema(description = "봉사자 ID", example = "123e4567-e89b-12d3-a456-426614174000")
@NotNull(message = "기관 ID는 필수값입니다.")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

봉사자 ID -> 기관 ID 오타인것 같아요.

그리고 UUID volunteerId은 로그인시 시큐리티에서 컨트롤러로 가져오는 값이지 않나요??
@Athunci~~ 어노테이션으로요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 실수를... 자세히 봐주셔서 감사합니다

7zrv added 2 commits November 29, 2024 23:19
- 봉사자 -> 기관으로 수정
- 기존 RequestMapping을 이용한 전역적인 엔드포인트 적용에서 엔드포인트마다 url을 붙여주는 방식으로 수정
@sonarqubecloud
Copy link

@7zrv 7zrv merged commit ff886b7 into main Nov 29, 2024
2 checks passed
@7zrv 7zrv deleted the feature/75-add-interest-center-create-delete branch November 29, 2024 14:26
ayoung-dev added a commit that referenced this pull request Nov 30, 2024
* fix(community): CommunityComment 엔티티 및 Dto에 communityBoardId 추가

* feat(community): CommunityBoardRepository에 existsById 추가 및 검증 테스트 추가

* feat(community): CommunityComment 생성 시 게시글 검증 로직 추가

* test(community): CommunityComment 생성 시 게시글 검증 테스트 추가

* test(community): CommunityCommentRepositoryTest 수정

* test(community): 불필요한 import 및 변수 삭제

* Feature/75 관심기관 등록, 취소 기능 구현 (#85)

* feat: 관심기관 등록 기능 구현

- InterestCenter 엔티티에 누락된 기관, 봉사자 id 필드 추가
- 관심 기관 등록 유스케이스 추가및 서비스 레이어 구현
- 등록, 검증을 위한 영속성 레이어 구현
- 중복 예외 클래스 추가
- 예외 메세지 추가
- 테스트 코드 작성및 검증 완

* feat: 관심기관 취소 기능 구현

- 관심 기관 취소 유스케이스 추가및 서비스 레이어 구현
- 관심기관 취소, 검증을 위한 영속성 레이어 구현
- 예외 메세지 추가
- 테스트 코드 작성및 검증 완료

* feat: 관심기관 API 컨트롤러 구현

- 관심 기관 등록과 취소 엔드포인트 컨트롤러 구현
- ApiResponse 공통 응답 객체 메서드 추가(논의 예정)
- 테스트 코드 작성및 검증완료

* feat: 요청 유효성 검증추가

- 관심기관 등록 요청 Dto에 유효성 검증 어노테이션 추가
- 활성화를 위해 컨트롤러에 @Valid 어노테이션 추가

* refactor: 공통 응답 객체 수정및 적용

- 기존 와일드카드를 사용하던 ApiResponse의 ok 메서드를 String 타입으로 리팩토링
- 변경에 따른 컨트롤러 return문 수정

* chore: 요청 Dto swagger scheme 수정

- 봉사자 -> 기관으로 수정

* chore: 컨트롤러 엔드 포인트 수정

- 기존 RequestMapping을 이용한 전역적인 엔드포인트 적용에서 엔드포인트마다 url을 붙여주는 방식으로 수정

* cicd: workflow 환경변수 추가 (#90)

- 기본 이미지 주소를 저장한 환경변수 값추가

* cicd: workflow 환경변수 추가 (#92)

- swagger에서의 테스트를 위한 환경변수 추가

* feat: 스웨거 API 테스트를 위한 봉사자 / 기관 토큰 발급 (#91)

* feat(jwt): JwtAuthFilter 필터 로직 수정
 - Authorization 없다면 익명 유저
 - Authorization 있다면 Bearer 인증 작업

* feat(swagger): 토큰 인증 추가

* feat(center): 이름 기반 조회 기능 추가

* feat(security): 디버깅을 위한 기관 봉사 토큰 발급 기능

* chore: 디버깅 env 추가

* chore: 불필요한 import 제거

* hotfix: fix typo (#94)

* fix(community): CommunityComment 엔티티 및 Dto에 communityBoardId 추가

* feat(community): CommunityBoardRepository에 existsById 추가 및 검증 테스트 추가

* feat(community): CommunityComment 생성 시 게시글 검증 로직 추가

* test(community): CommunityComment 생성 시 게시글 검증 테스트 추가

* test(community): CommunityCommentRepositoryTest 수정

* test(community): 불필요한 import 및 변수 삭제

* refactor(community): 코드 리뷰 사항 반영

- existsById 메서드 QeuryDSL -> JPA
- repositoryTest 빌더 beforeEach로 추출
- serviceTest 예외 메세지 static import
- serviceTest usecase 주입 > repository로 주입

* test(community): 변수명 변경

---------

Co-authored-by: seojin Yoon <[email protected]>
Co-authored-by: Beom-Su Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants